Code cleanup & Use Flow.Launcher.Localization to improve code quality #4009
Conversation
|
🥷 Code experts: no user but you matched threshold 10 Jack251970, jjw24 have most 🧠 knowledge in the files. See details
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors several plugins to use a static PluginInitContext and centralized Localize-based i18n. Adds Flow.Launcher.Localization dependency and suppresses FLSG0007 in multiple csproj files. ProcessKiller and Sys plugins receive substantial structural updates and new/static utilities. Shell plugin internal flow adjusted. Minor formatting changes in Calculator. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FL as Flow Launcher
participant PK as ProcessKiller.Main (static Context)
participant PH as ProcessHelper (static)
Note over FL,PK: Process search and kill flow
User->>FL: Enter "kill <query>"
FL->>PK: Query(query)
PK->>PH: GetMatchingProcesses()/GetProcessesWithNonEmptyWindowTitle()
PH-->>PK: Processes + titles
PK->>PK: Fuzzy match, group similar, insert "Kill all" when applicable
PK-->>FL: List<Result>
User->>FL: Select result
FL->>PH: TryKill(process) or for group: TryKill(each)
PH-->>FL: Success/Failure
FL->>PK: ReQuery()
sequenceDiagram
autonumber
actor User
participant FL as Flow Launcher
participant SYS as Sys.Main (static Context)
participant TS as ThemeSelector (public static)
Note over FL,SYS: System commands and theme selection
User->>FL: Enter "fltheme ..." or "sys cmd"
alt Theme selection
FL->>TS: Query(query)
TS->>FL: Theme results (with Localize subtitles)
User->>FL: Pick theme
FL->>TS: Action (SetCurrentTheme)
TS->>SYS: Context.API.SetCurrentTheme(...)
SYS->>FL: Context.API.ReQuery()
else System commands
FL->>SYS: Query(query)
SYS->>SYS: Build command list, fuzzy match
SYS-->>FL: Results (titles/subtitles via Localize)
User->>FL: Execute command
FL->>SYS: Action (via Context.API)
end
sequenceDiagram
autonumber
actor User
participant FL as Flow Launcher
participant SH as Shell.Main (static Context)
Note over FL,SH: Shell command execution
User->>FL: Type shell path/cmd
FL->>SH: Query(query)
SH->>SH: Resolve history/autocomplete, Localize titles
SH-->>FL: Results
User->>FL: Execute
FL->>SH: Execute(result)
SH->>SH: Prepare ProcessStartInfo
SH-->>FL: Start or error (localized)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a significant modernization of the Flow.Launcher plugin codebase by adopting the Flow.Launcher.Localization package and C# file-scoped namespace syntax. The changes eliminate direct API translation calls in favor of generated localization methods and apply consistent modern C# patterns throughout.
- Replaces
_context.API.GetTranslation()calls withLocalizemethods for type-safe localization - Adopts file-scoped namespaces and modern C# syntax patterns (primary constructors, collection expressions)
- Refactors static access patterns by converting instance context members to static properties
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher.Plugin.Sys/ThemeSelector.cs | Converts to static class with file-scoped namespace, replaces localization calls |
| Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs | Removes context parameter, adopts file-scoped namespace |
| Flow.Launcher.Plugin.Sys/SettingsViewModel.cs | Converts to primary constructor pattern |
| Flow.Launcher.Plugin.Sys/Settings.cs | Updates collection initialization to modern syntax |
| Flow.Launcher.Plugin.Sys/Main.cs | Major refactor to static context access and Localize usage |
| Flow.Launcher.Plugin.Sys/Languages/en.xaml | Adds new theme selector localization strings |
| Flow.Launcher.Plugin.Sys/Flow.Launcher.Plugin.Sys.csproj | Adds Flow.Launcher.Localization package reference |
| Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml.cs | Updates to use static context and Localize methods |
| Flow.Launcher.Plugin.Shell/Main.cs | Converts to static context pattern and modern syntax |
| Flow.Launcher.Plugin.ProcessKiller/* | Multiple files modernized with static context and localization updates |
| Flow.Launcher.Plugin.PluginIndicator/Main.cs | Refactored for static context and modern patterns |
| Flow.Launcher.Plugin.Calculator/* | Minor modernization updates |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
No feature updates and good to go |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)
61-67: Scheme handling bug: ftp (and other schemes) get prefixed with http://.If the user enters ftp://host, this code creates http://ftp://host. Prefer checking for any scheme and defaulting to https only when missing.
- if (!raw.ToLower().StartsWith("http")) - { - raw = "http://" + raw; - } + if (!raw.Contains("://", StringComparison.Ordinal)) + { + raw = "https://" + raw; + }
🧹 Nitpick comments (16)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (4)
43-43: Static Context: add null-forgiving initializer to satisfy NRT.Prevents nullable warnings while keeping the private setter.
-internal static PluginInitContext Context { get; private set; } +internal static PluginInitContext Context { get; private set; } = null!;
42-42: Make the regex field static readonly.Avoids per-instance allocation and signals immutability.
- Regex reg = new Regex(urlPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex reg = new(urlPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase);
83-90: Avoid culture-sensitive ToLower and use case-insensitive comparisons.The regex is already IgnoreCase; prefer OrdinalIgnoreCase for the localhost checks.
- raw = raw.ToLower(); - - if (reg.Match(raw).Value == raw) return true; + if (reg.IsMatch(raw)) return true; - if (raw == "localhost" || raw.StartsWith("localhost:") || - raw == "http://localhost" || raw.StartsWith("http://localhost:") || - raw == "https://localhost" || raw.StartsWith("https://localhost:") + if (string.Equals(raw, "localhost", StringComparison.OrdinalIgnoreCase) || + raw.StartsWith("localhost:", StringComparison.OrdinalIgnoreCase) || + raw.StartsWith("http://localhost", StringComparison.OrdinalIgnoreCase) || + raw.StartsWith("https://localhost", StringComparison.OrdinalIgnoreCase) )
73-75: Log the exception before returningIPublicAPI exposes LogException — log the caught exception and keep the localized user-facing error.
- catch(Exception) - { - Context.API.ShowMsgError(Localize.flowlauncher_plugin_url_cannot_open_url(raw)); - return false; - } + catch (Exception ex) + { + Context.API.LogException(nameof(Flow.Launcher.Plugin.Url.Main), "Failed to open URL", ex); + Context.API.ShowMsgError(Localize.flowlauncher_plugin_url_cannot_open_url(raw)); + return false; + }Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs (4)
29-29: Prevent stale LeaveShellOpen when Shell = RunCommandWhen RunCommand is selected, also clear the checkbox and setting to avoid inconsistent state.
- LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; + LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; + if (_settings.Shell == Shell.RunCommand) + { + LeaveShellOpen.IsChecked = false; + _settings.LeaveShellOpen = false; + } @@ - LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; + LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; + if (_settings.Shell == Shell.RunCommand) + { + LeaveShellOpen.IsChecked = false; + _settings.LeaveShellOpen = false; + }Also applies to: 118-119
11-15: Null‑guard the settings dependencyMinor safety: fail fast if null is passed.
+using System; using System.Collections.Generic; @@ - _settings = settings; + _settings = settings ?? throw new ArgumentNullException(nameof(settings));
101-107: Decouple Shell mapping from ComboBox index orderIndex-based mapping is brittle if XAML item order changes. Prefer enum ItemsSource + SelectedItem.
- ShellComboBox.SelectedIndex = _settings.Shell switch - { - Shell.Cmd => 0, - Shell.Powershell => 1, - Shell.Pwsh => 2, - _ => ShellComboBox.Items.Count - 1 - }; + ShellComboBox.ItemsSource = new[] { Shell.Cmd, Shell.Powershell, Shell.Pwsh, Shell.RunCommand }; + ShellComboBox.SelectedItem = _settings.Shell; @@ - ShellComboBox.SelectionChanged += (o, e) => - { - _settings.Shell = ShellComboBox.SelectedIndex switch - { - 0 => Shell.Cmd, - 1 => Shell.Powershell, - 2 => Shell.Pwsh, - _ => Shell.RunCommand - }; - LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; - }; + ShellComboBox.SelectionChanged += (o, e) => + { + _settings.Shell = (Shell)ShellComboBox.SelectedItem; + LeaveShellOpen.IsEnabled = _settings.Shell != Shell.RunCommand; + };If XAML already defines items, treat this as a future cleanup.
Also applies to: 109-119
71-99: Reduce repetitive Checked/Unchecked boilerplateOptional: introduce a small helper to bind a CheckBox to a bool setting to cut duplication.
Example:
private static void Bind(CheckBox cb, Action<bool> set) { cb.Checked += (_, __) => set(true); cb.Unchecked += (_, __) => set(false); } // usage: Bind(AlwaysRunAsAdministrator, v => _settings.RunAsAdministrator = v); Bind(UseWindowsTerminal, v => _settings.UseWindowsTerminal = v); Bind(ReplaceWinR, v => _settings.ReplaceWinR = v);Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (3)
20-25: Guard against null settings.LoadSettingJsonStorage can return null; ensure a default Settings instance.
- _settings = context.API.LoadSettingJsonStorage<Settings>(); + _settings = context.API.LoadSettingJsonStorage<Settings>() ?? new Settings();
72-79: Prefer returning an empty list over null.Returning null hides the plugin entirely; returning an empty list avoids NREs at call sites and is usually safer.
- return null; + return new List<Result>(0);Please confirm Flow Launcher’s expected behavior for “no results” here; if the project consistently uses null, keep the current approach for consistency.
72-159: Optional: avoid holding Process handles in results.To reduce handle pressure, store Process Id (and name/path) in ProcessResult, and reacquire Process by Id inside Action.
I can draft a targeted refactor if desired.
Plugins/Flow.Launcher.Plugin.Url/Flow.Launcher.Plugin.Url.csproj (1)
36-36: Be cautious suppressing FLSG0007 only in Release; prefer fixing or narrowly scoping the suppression.
- Suppressing analyzer rule FLSG0007 only in Release risks divergence between Debug/Release behaviors and can hide issues. If the warning is legitimate, fix at source; if it’s noisy for a specific file, scope the suppression to that file rather than the whole project/config.
Option A (scoped suppression for a specific file) — keep project‑wide quality intact:
<ItemGroup> <!-- Example: scope the suppression to a single file that cannot be changed --> <Compile Update="Path\To\The\File.cs"> <NoWarn>$(NoWarn);FLSG0007</NoWarn> </Compile> </ItemGroup>If this suppression is intentional across many plugins, centralize it with justification in Directory.Build.props instead of per‑csproj to avoid drift.
Would you add a short comment (reason/issue link) next to this NoWarn so future maintainers know why it’s needed?
Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (4)
11-26: Relevance lost: fuzzy score ignored and results sorted alphabetically.You compute fuzzy scores but then (1) overwrite them in CreateThemeResult and (2) sort by Title. Preserve match-based ranking and sort by Score (desc), with Title as a tiebreaker.
Apply this diff to the two returns:
- return [.. themes.Select(x => CreateThemeResult(x, selectedTheme)).OrderBy(x => x.Title)]; + return [.. themes + .Select(x => CreateThemeResult(x, selectedTheme)) + .OrderByDescending(x => x.Score) + .ThenBy(x => x.Title)]; - return [.. themes.Select(theme => (theme, matchResult: Main.Context.API.FuzzySearch(search, theme.Name))) - .Where(x => x.matchResult.IsSearchPrecisionScoreMet()) - .Select(x => CreateThemeResult(x.theme, selectedTheme, x.matchResult.Score, x.matchResult.MatchData)) - .OrderBy(x => x.Title)]; + return [.. themes + .Select(theme => (theme, matchResult: Main.Context.API.FuzzySearch(search, theme.Name))) + .Where(x => x.matchResult.IsSearchPrecisionScoreMet()) + .Select(x => CreateThemeResult(x.theme, selectedTheme, x.matchResult.Score, x.matchResult.MatchData)) + .OrderByDescending(x => x.Score) + .ThenBy(x => x.Title)];
30-44: Don’t overwrite the fuzzy score; carry it into Result.Score.Currently all non-current themes end up at Score=1000. Use 1000 + matchScore and keep current theme at 2000.
-private static Result CreateThemeResult(ThemeData theme, ThemeData selectedTheme, int score, IList<int> highlightData) +private static Result CreateThemeResult(ThemeData theme, ThemeData selectedTheme, int matchScore, IList<int> highlightData) { - string title; - if (theme == selectedTheme) - { - title = $"{theme.Name} ★"; - // Set current theme to the top - score = 2000; - } - else - { - title = theme.Name; - // Set them to 1000 so that they are higher than other non-theme records - score = 1000; - } + var isCurrent = theme == selectedTheme; + var title = isCurrent ? $"{theme.Name} ★" : theme.Name; + // Keep current theme on top; otherwise use fuzzy score for relevance + var finalScore = isCurrent ? 2000 : 1000 + matchScore; @@ - { - Title = title, + { + Title = title, TitleHighlightData = highlightData, SubTitle = description, IcoPath = "Images\\theme_selector.png", Glyph = new GlyphInfo("/Resources/#Segoe Fluent Icons", "\ue790"), - Score = score, + Score = finalScore,Also applies to: 70-78
46-68: Description logic fix looks good; optional simplification.The redundant HasBlur check is gone. Consider a compact switch expression for readability.
- string description; - if (theme.IsDark == true) - { - if (theme.HasBlur == true) - { - description = Localize.flowlauncher_plugin_sys_type_isdark_hasblur(); - } - else - { - description = Localize.flowlauncher_plugin_sys_type_isdark(); - } - } - else - { - if (theme.HasBlur == true) - { - description = Localize.flowlauncher_plugin_sys_type_hasblur(); - } - else - { - description = string.Empty; - } - } + var description = (theme.IsDark, theme.HasBlur) switch + { + (true, true) => Localize.flowlauncher_plugin_sys_type_isdark_hasblur(), + (true, _) => Localize.flowlauncher_plugin_sys_type_isdark(), + (_, true) => Localize.flowlauncher_plugin_sys_type_hasblur(), + _ => string.Empty + };
75-75: Use forward slashes in IcoPath.Avoid Windows-only backslashes and escaping.
- IcoPath = "Images\\theme_selector.png", + IcoPath = "Images/theme_selector.png",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Url/Flow.Launcher.Plugin.Url.csproj(2 hunks)Plugins/Flow.Launcher.Plugin.Url/Main.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Plugins/Flow.Launcher.Plugin.Url/Flow.Launcher.Plugin.Url.csproj
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.Url/Flow.Launcher.Plugin.Url.csproj
📚 Learning: 2025-09-23T04:39:31.971Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#4009
File: Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs:135-139
Timestamp: 2025-09-23T04:39:31.971Z
Learning: In Flow.Launcher plugin settings UI code, setting ComboBox.SelectedItem directly to an integer value after populating ItemsSource with a List<int> works correctly and doesn't require additional validation checks. User Jack251970 confirmed this pattern is acceptable in the codebase.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Plugins/Flow.Launcher.Plugin.Url/Main.cs
📚 Learning: 2025-01-18T10:10:18.414Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Applied to files:
Plugins/Flow.Launcher.Plugin.Url/Main.cs
🧬 Code graph analysis (4)
Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (4)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (3)
List(66-110)List(196-513)Main(18-529)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (5)
List(177-177)List(445-445)ThemeData(451-451)SetCurrentTheme(460-460)ReQuery(399-399)Flow.Launcher.Core/Resource/Theme.cs (3)
List(396-409)ThemeData(333-365)ThemeData(385-394)Flow.Launcher.Plugin/SharedModels/ThemeData.cs (2)
ThemeData(8-77)ThemeData(33-39)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (5)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Settings.cs (1)
Settings(3-8)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs (5)
ProcessHelper(15-186)List(58-70)TryKill(145-159)GetProcessNameIdTitle(46-53)TryGetProcessFilename(161-185)Plugins/Flow.Launcher.Plugin.ProcessKiller/ViewModels/SettingsViewModel.cs (1)
SettingsViewModel(3-6)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessResult.cs (1)
ProcessResult(6-17)Plugins/Flow.Launcher.Plugin.ProcessKiller/Views/SettingsControl.xaml.cs (2)
SettingsControl(6-14)SettingsControl(8-13)
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs (1)
Plugins/Flow.Launcher.Plugin.Shell/Settings.cs (1)
Settings(5-32)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (3)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
GetTranslatedPluginTitle(429-432)GetTranslatedPluginDescription(434-437)Plugins/Flow.Launcher.Plugin.Program/Main.cs (2)
GetTranslatedPluginTitle(375-378)GetTranslatedPluginDescription(380-383)Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (2)
GetTranslatedPluginTitle(213-216)GetTranslatedPluginDescription(218-221)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs
[warning] 198-198:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 197-197:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 150-150:
processlist is not a recognized word. (unrecognized-spelling)
[warning] 136-136:
processlist is not a recognized word. (unrecognized-spelling)
[warning] 107-107:
processlist is not a recognized word. (unrecognized-spelling)
[warning] 98-98:
processlist is not a recognized word. (unrecognized-spelling)
[warning] 54-54:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 39-39:
processkiller is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (12)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (4)
56-56: Good switch to centralized localization for subtitle.Consistent with other plugins using Flow.Launcher.Localization.
100-105: Init assignment LGTM.Static Context assignment aligns with repo patterns; PluginInitContext is guaranteed non-null by the plugin system.
109-110: Localized plugin title LGTM.Matches the new localization approach.
114-115: Localized plugin description LGTM.Consistent with other plugins.
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs (3)
33-34: Nice null‑safe checkSwitch to
IsChecked != trueis correct and clearer. Matches prior guidance.
36-43: Good defaulting of MostUsed countItemsSource set before selecting, and zero maps to first option and persists to settings. Solid.
135-139: SelectedItem assignment to int is acceptable herePer retrieved learnings for this codebase, assigning the int directly after populating
List<int>is fine.Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (1)
32-40: Localization migration LGTM.Using Flow.Launcher.Localization APIs for title/description is correct and consistent.
Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (4)
80-85: Confirm intended UX for Action’s return value.Returning false keeps the query window open after theme change (even with ReQuery). If you prefer closing, return true.
7-9: Static helper and keyword extraction: LGTM.Good move to make this a focused static helper with a single entry keyword.
28-29: Overload that delegates to the detailed helper: LGTM.Keeps call sites terse for the no-search path.
19-26: Resolved — project already targets SDK with C#12+ supportAll csproj TargetFrameworks are net9.0 and global.json pins SDK "9.0.*"; no explicit LangVersion/UsePreviewLanguage found, so the compiler will use the SDK-supported language (C# 12+). The [..] collection-expression is supported — no changes required.
| namespace Flow.Launcher.Plugin.Calculator; | ||
|
|
||
| [EnumLocalize] | ||
| public enum DecimalSeparator |
There was a problem hiding this comment.
Personally I don't like a change like this. It just removes braces and indent and messes up commit history.
There was a problem hiding this comment.
Personally I don't like a change like this. It just removes braces and indent and messes up commit history.
That makes sense. Let us revert it.
There was a problem hiding this comment.
@VictoriousRaptor All related namespaces are reverted
|
Don't have much time today. If someone can merge it please do squash and merge. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs (2)
19-35: Make system-process check culture/Case-insensitive and drop ToLower().Use a HashSet with OrdinalIgnoreCase and compare ProcessName directly.
- private readonly HashSet<string> _systemProcessList = - [ + private readonly HashSet<string> _systemProcessList = new(StringComparer.OrdinalIgnoreCase) + { "conhost", "svchost", "idle", "system", "rundll32", "csrss", "lsass", "lsm", "smss", "wininit", "winlogon", "services", "spoolsv", - "explorer" - ]; + "explorer" + }; ... - private bool IsSystemProcessOrFlowLauncher(Process p) => - _systemProcessList.Contains(p.ProcessName.ToLower()) || + private bool IsSystemProcessOrFlowLauncher(Process p) => + _systemProcessList.Contains(p.ProcessName) || string.Equals(p.ProcessName, FlowLauncherProcessName, StringComparison.OrdinalIgnoreCase);Also applies to: 39-41
165-171: Wrong handle null check type. Use HANDLE.Null, not HWND.Null.This is a correctness/clarity bug; check against the correct handle type.
- var handle = PInvoke.OpenProcess(PROCESS_ACCESS_RIGHTS.PROCESS_QUERY_LIMITED_INFORMATION, false, (uint)p.Id); - if (handle == HWND.Null) + var handle = PInvoke.OpenProcess(PROCESS_ACCESS_RIGHTS.PROCESS_QUERY_LIMITED_INFORMATION, false, (uint)p.Id); + if (handle == HANDLE.Null) { return string.Empty; }Also applies to: 175-181
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
63-68: Remove duplicate Any() check in autocomplete filter.Redundant predicate repeats the same condition.
- .Where(o => o.StartsWith(cmd, StringComparison.OrdinalIgnoreCase) && - !results.Any(p => o.Equals(p.Title, StringComparison.OrdinalIgnoreCase)) && - !results.Any(p => o.Equals(p.Title, StringComparison.OrdinalIgnoreCase))).ToList(); + .Where(o => o.StartsWith(cmd, StringComparison.OrdinalIgnoreCase) && + !results.Any(p => o.Equals(p.Title, StringComparison.OrdinalIgnoreCase))) + .ToList();
218-229: Fix injected cmd string: stray “/c” after pause.Constructs “… && pause > nul /c”, which yields “‘/c’ is not recognized …”. Remove the trailing “/c”.
- info.ArgumentList.Add( - $"{command}" + - $"{(_settings.CloseShellAfterPress ? - $" && echo {notifyStr} && pause > nul /c" : - "")}"); + info.ArgumentList.Add( + $"{command}" + + $"{(_settings.CloseShellAfterPress ? + $" && echo {notifyStr} && pause > nul" : + "")}");Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (1)
33-38: Use value equality for selected theme, not reference equality.GetCurrentTheme() returns a different instance than items in themes; ‘==’ will rarely match. Compare by identifier.
- if (theme == selectedTheme) + if (string.Equals(theme.FileNameWithoutExtension, selectedTheme?.FileNameWithoutExtension, StringComparison.OrdinalIgnoreCase)) { title = $"{theme.Name} ★"; // Set current theme to the top score = 2000; }Plugins/Flow.Launcher.Plugin.Sys/Main.cs (1)
134-144: Initialize localized command metadata before first query
UpdateLocalizedNameDescriptionis only invoked fromCreateSettingPanel, so until the settings UI is opened everyCommand.NameandCommand.Descriptionstaysnull. During the first search we overwrite eachResult.Titlewith that null, then immediately callContext.API.FuzzySearchwith a null string, which blows up (and even if it didn’t, the UI would render blank titles). Please hydrate the localized names insideInitright after loading_settingsso that every query runs against non-null data.Context = context; _settings = context.API.LoadSettingJsonStorage<Settings>(); _viewModel = new SettingsViewModel(_settings); + UpdateLocalizedNameDescription(force: false); foreach (string key in KeywordTitleMappings.Keys) { // Remove _cmd in the last of the strings KeywordDescriptionMappings[key] = KeywordTitleMappings[key][..^4]; }
🧹 Nitpick comments (3)
Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessResult.cs (1)
6-17: Primary constructor refactor LGTM; consider sealing the classThe refactor is clean. If inheritance isn’t needed, mark the class
sealedfor clarity and minor JIT benefits.-internal class ProcessResult(Process process, int score, string title, MatchResult match, string tooltip) +internal sealed class ProcessResult(Process process, int score, string title, MatchResult match, string tooltip)Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (1)
186-188: Preserve scoring when ordering results.Sorting solely by Title discards the fuzzy/visibility Score. Sort by Score then Title.
- var sortedResults = results.OrderBy(x => x.Title).ToList(); + var sortedResults = results + .OrderByDescending(x => x.Score) + .ThenBy(x => x.Title) + .ToList();Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (1)
19-20: Honor score when listing themes with no search.Sort by Score then Title so current theme surfaces as intended.
- return [.. themes.Select(x => CreateThemeResult(x, selectedTheme)).OrderBy(x => x.Title)]; + return [.. themes + .Select(x => CreateThemeResult(x, selectedTheme)) + .OrderByDescending(x => x.Score) + .ThenBy(x => x.Title)];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Plugins/Flow.Launcher.Plugin.Calculator/DecimalSeparator.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(8 hunks)Plugins/Flow.Launcher.Plugin.PluginIndicator/Main.cs(4 hunks)Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs(10 hunks)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs(4 hunks)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessResult.cs(1 hunks)Plugins/Flow.Launcher.Plugin.ProcessKiller/ViewModels/SettingsViewModel.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Shell/Main.cs(18 hunks)Plugins/Flow.Launcher.Plugin.Shell/Settings.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Sys/Main.cs(21 hunks)Plugins/Flow.Launcher.Plugin.Sys/SettingsViewModel.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- Plugins/Flow.Launcher.Plugin.Calculator/DecimalSeparator.cs
- Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml.cs
- Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs
- Plugins/Flow.Launcher.Plugin.ProcessKiller/ViewModels/SettingsViewModel.cs
- Plugins/Flow.Launcher.Plugin.Sys/SettingsViewModel.cs
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3573
File: Plugins/Flow.Launcher.Plugin.Shell/Main.cs:330-339
Timestamp: 2025-09-06T05:32:51.575Z
Learning: In Flow.Launcher Shell plugin's StartProcess method, the maintainer Jack251970 prefers not to propagate launch failures from Context.API.StartProcess or throw exceptions when the API call returns false. Silent failure handling is intentional for shell commands in this plugin.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
📚 Learning: 2025-09-23T04:39:31.981Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#4009
File: Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs:135-139
Timestamp: 2025-09-23T04:39:31.981Z
Learning: In Flow.Launcher plugin settings UI code, setting ComboBox.SelectedItem directly to an integer value after populating ItemsSource with a List<int> works correctly and doesn't require additional validation checks. User Jack251970 confirmed this pattern is acceptable in the codebase.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs
📚 Learning: 2025-01-18T10:10:18.414Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Applied to files:
Plugins/Flow.Launcher.Plugin.Sys/Main.csPlugins/Flow.Launcher.Plugin.PluginIndicator/Main.csPlugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Applied to files:
Plugins/Flow.Launcher.Plugin.Sys/Main.csPlugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Plugins/Flow.Launcher.Plugin.Sys/Main.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Plugins/Flow.Launcher.Plugin.PluginIndicator/Main.csPlugins/Flow.Launcher.Plugin.Calculator/Main.csPlugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs
📚 Learning: 2025-09-06T05:32:51.575Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3573
File: Plugins/Flow.Launcher.Plugin.Shell/Main.cs:330-339
Timestamp: 2025-09-06T05:32:51.575Z
Learning: In Flow.Launcher Shell plugin's StartProcess method, the maintainer Jack251970 prefers not to propagate launch failures from Context.API.StartProcess or throw exceptions when the API call returns false. Silent failure handling is intentional for shell commands in this plugin.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2025-09-13T12:50:05.306Z
Learnt from: dcog989
PR: Flow-Launcher/Flow.Launcher#3971
File: Plugins/Flow.Launcher.Plugin.Calculator/Main.cs:67-67
Timestamp: 2025-09-13T12:50:05.306Z
Learning: NumberRegex pattern `-?[\d\.,]+` in Flow.Launcher Calculator will match function arguments like `2,3` in `min(2,3)`. The NormalizeNumber validation that checks for proper 3-digit thousand separator grouping is essential to avoid breaking function calls by incorrectly stripping commas from function arguments.
Applied to files:
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs
🧬 Code graph analysis (5)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (4)
Flow.Launcher/PublicAPIInstance.cs (19)
System(159-222)List(251-251)List(530-530)LogError(282-283)GetTranslation(249-249)HideMainWindow(96-96)SaveAppAllSettings(108-117)ShowMsg(127-128)ShowMsg(130-133)RestartApp(77-90)OpenSettingDialog(143-149)CheckForNewUpdate(106-106)GetLogDirectory(622-622)OpenDirectory(330-410)OpenUrl(475-478)OpenUrl(480-483)GetDataDirectory(620-620)ToggleGameMode(495-498)ChangeQuery(72-75)Plugins/Flow.Launcher.Plugin.Sys/Settings.cs (2)
Settings(6-127)Settings(8-14)Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs (2)
SysSettings(7-51)SysSettings(11-16)Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (4)
List(11-26)Result(28-28)Result(30-87)ThemeSelector(7-88)
Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (4)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (3)
List(66-110)List(196-513)Main(18-529)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (5)
List(177-177)List(445-445)ThemeData(451-451)SetCurrentTheme(460-460)ReQuery(399-399)Flow.Launcher.Core/Resource/Theme.cs (3)
List(396-409)ThemeData(333-365)ThemeData(385-394)Flow.Launcher.Plugin/SharedModels/ThemeData.cs (2)
ThemeData(8-77)ThemeData(33-39)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs (3)
List(24-24)List(26-72)List(74-86)Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (2)
Result(28-28)Result(30-87)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (1)
Result(397-473)
Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs (1)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (1)
Main(10-220)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (6)
Plugins/Flow.Launcher.Plugin.ProcessKiller/Settings.cs (1)
Settings(3-8)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs (4)
ProcessHelper(15-186)TryKill(145-159)GetProcessNameIdTitle(46-53)TryGetProcessFilename(161-185)Plugins/Flow.Launcher.Plugin.ProcessKiller/ViewModels/SettingsViewModel.cs (1)
SettingsViewModel(3-6)Plugins/Flow.Launcher.Plugin.Sys/Main.cs (2)
Init(134-144)GetTranslatedPluginDescription(520-523)Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Init(34-47)GetTranslatedPluginDescription(417-420)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessResult.cs (1)
ProcessResult(6-17)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs
[warning] 39-39:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 54-54:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 136-136:
processlist is not a recognized word. (unrecognized-spelling)
[warning] 198-198:
processkiller is not a recognized word. (unrecognized-spelling)
[warning] 197-197:
processkiller is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
Plugins/Flow.Launcher.Plugin.Shell/ShellSetting.xaml.cs (2)
33-35: Null-safe check LGTMThe switch to
ShowOnlyMostUsedCMDs.IsChecked != trueis correct and clearer.
17-140: Guard against duplicate event subscriptions in LoadedLoaded can fire multiple times. Wrap handler wiring so it runs once to avoid stacked subscriptions.
Apply minimally:
public partial class CMDSetting : UserControl { private readonly Settings _settings; + private bool _eventsWired; @@ private void CMDSetting_OnLoaded(object sender, RoutedEventArgs re) { @@ - CloseShellAfterPress.Checked += (o, e) => - { - _settings.CloseShellAfterPress = true; - LeaveShellOpen.IsChecked = false; - LeaveShellOpen.IsEnabled = false; - }; + if (!_eventsWired) + { + CloseShellAfterPress.Checked += (o, e) => + { + _settings.CloseShellAfterPress = true; + LeaveShellOpen.IsChecked = false; + LeaveShellOpen.IsEnabled = false; + }; @@ - ShowOnlyMostUsedCMDsNumber.SelectionChanged += (o, e) => - { - _settings.ShowOnlyMostUsedCMDsNumber = (int)ShowOnlyMostUsedCMDsNumber.SelectedItem; - }; + ShowOnlyMostUsedCMDsNumber.SelectionChanged += (o, e) => + { + _settings.ShowOnlyMostUsedCMDsNumber = (int)ShowOnlyMostUsedCMDsNumber.SelectedItem; + }; + _eventsWired = true; + }Plugins/Flow.Launcher.Plugin.Shell/Settings.cs (4)
23-23: Modern collection expression LGTM
= [];is fine (C# 12).
27-31: TryAdd pattern LGTMClear and thread-safe enough for typical usage.
11-14: Consolidate opposing flags to a single source of truth
CloseShellAfterPressvsLeaveShellOpencan drift. Consider one field with an obsolete shim for back-compat.- public bool CloseShellAfterPress { get; set; } = false; - public bool LeaveShellOpen { get; set; } + public bool LeaveShellOpen { get; set; } + [System.Obsolete("Use LeaveShellOpen.", error: false)] + public bool CloseShellAfterPress + { + get => !LeaveShellOpen; + set => LeaveShellOpen = !value; + }
34-40: Brand casing: add PowerShell alias without breaking valuesKeep existing
Powershellfor compat; addPowerShell = Powershellalias.public enum Shell { Cmd = 0, - Powershell = 1, + Powershell = 1, + PowerShell = Powershell, RunCommand = 2, Pwsh = 3, }Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
29-29: Private setter for Context LGTMImproves encapsulation and prevents accidental reassignment.
239-241: Fix comment: log10 is base-10, not natural logUpdate the comment to avoid confusion.
- // log(x) -> log10(x) (natural log) + // log(x) -> log10(x) (base-10 log)Plugins/Flow.Launcher.Plugin.ProcessKiller/ProcessHelper.cs (1)
140-143: Guard empty paths and compare file paths case‑insensitively (Windows).Prevents accidental broad matches and aligns with OS semantics.
- public IEnumerable<Process> GetSimilarProcesses(string processPath) - { - return Process.GetProcesses().Where(p => !IsSystemProcessOrFlowLauncher(p) && TryGetProcessFilename(p) == processPath); - } + public IEnumerable<Process> GetSimilarProcesses(string processPath) + { + if (string.IsNullOrWhiteSpace(processPath)) + return Enumerable.Empty<Process>(); + + return Process.GetProcesses().Where(p => + !IsSystemProcessOrFlowLauncher(p) && + string.Equals(TryGetProcessFilename(p), processPath, StringComparison.OrdinalIgnoreCase)); + }Plugins/Flow.Launcher.Plugin.ProcessKiller/Main.cs (2)
47-66: Don’t batch‑kill on empty path; materialize similar processes once.Avoids grouping unrelated items when SubTitle is empty and prevents double enumeration.
var menuOptions = new List<Result>(); var processPath = result.SubTitle; - // get all non-system processes whose file path matches that of the given result (processPath) - var similarProcesses = processHelper.GetSimilarProcesses(processPath); + if (string.IsNullOrWhiteSpace(processPath)) + return menuOptions; + + // get all non-system processes whose file path matches that of the given result (processPath) + var similarProcesses = processHelper.GetSimilarProcesses(processPath).ToList(); - if (similarProcesses.Any()) + if (similarProcesses.Count > 0) {
191-211: “Kill all” can appear without a valid path and target unrelated processes.Require non‑empty anchor path and use OrdinalIgnoreCase comparison.
- var firstResult = sortedResults.FirstOrDefault(x => !string.IsNullOrEmpty(x.SubTitle)); - if (processlist.Count > 1 && !string.IsNullOrEmpty(searchTerm) && sortedResults.All(r => r.SubTitle == firstResult?.SubTitle)) + var firstResult = sortedResults.FirstOrDefault(x => !string.IsNullOrEmpty(x.SubTitle)); + if (processlist.Count > 1 + && !string.IsNullOrEmpty(searchTerm) + && firstResult is not null + && !string.IsNullOrEmpty(firstResult.SubTitle) + && sortedResults.All(r => string.Equals(r.SubTitle, firstResult.SubTitle, StringComparison.OrdinalIgnoreCase))) {Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
20-20: Initialize static Context to quiet nullability warnings.Matches pattern used elsewhere; avoids pre‑Init analyzer noise.
- internal static PluginInitContext Context { get; private set; } + internal static PluginInitContext Context { get; private set; } = null!;Plugins/Flow.Launcher.Plugin.PluginIndicator/Main.cs (2)
8-8: Initialize static Context to avoid nullability warnings.Consistent with other plugins.
- internal static PluginInitContext Context { get; private set; } + internal static PluginInitContext Context { get; private set; } = null!;
20-48: LGTM on localization and static Context usage.QueryResults pipeline and Localize.* integration look correct.
jjw24
added
bug
Code cleanup & Use
Flow.Launcher.Localizationto improve code qualityNo feature updates.